Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Electron onSendError callbacks not being called #1999

Merged
merged 6 commits into from
Aug 4, 2023

Conversation

imjoehaines
Copy link
Contributor

Goal

The onSendError configuration option was not being passed down to the minidump delivery loop and so was not being called

Were this to work, it would also have been passed the an Event in its JSON serialised form rather than a real Event object (see equivalent option in Android & Cocoa)

This PR fixes the bug causing onSendError to not be applied and deserialises the Event JSON into an Event object

Using 'runSyncCallbacks' from core ensures the behaviour of onSendError
callbacks matches onError callbacks

Notably errors will be caught, logged and won't stop subsequent
callbacks from being called

The callback array is now treated like a stack, i.e. the most recently
added callback is called first (LIFO) rather than last (FIFO/a queue)
This allows using the Event API (addMetadata etc...) inside the
callback. Otherwise users would have to know the internal JSON
representation of an Event in order to mutate it
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 43.65 kB 13.39 kB
After 43.65 kB 13.39 kB
± No change No change

code coverage diff

<temporarily disabled>

Generated by 🚫 dangerJS against c840061

@imjoehaines imjoehaines merged commit bef6918 into next Aug 4, 2023
12 checks passed
@imjoehaines imjoehaines deleted the electron-on-send-error branch August 4, 2023 07:36
@yousif-bugsnag yousif-bugsnag mentioned this pull request Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants